Add clang tidy performance checks #8429
Conversation
| RegistryEntry{ | ||
| std::move(animationsVector), | ||
| buildAnimationToIndexMap(animationsVector)}); | ||
| animationsVector, buildAnimationToIndexMap(animationsVector)}); |
There was a problem hiding this comment.
I think that we should keep std::move here and just remove the const modifier from the animationsVector declaration:
const auto animationsVector =
buildAnimationsVector(rt, shadowNode, animationNames, newAnimations);This std::move call was added purposely but the the const modifier near the animationsVector prevented it from working as expected.
There was a problem hiding this comment.
What I don't get here is why do we move the animationsVector first and then try to use it straight away. Isn't it in indeterminate state after that?
There was a problem hiding this comment.
I see your point. Maybe, to be extra safe, we could move these registry updates below function calls that use the animationsVector directly to get something like this in the end:
auto animationsVector =
buildAnimationsVector(rt, shadowNode, animationNames, newAnimations);
const auto viewTag = shadowNode->getTag();
if (animationsVector.empty()) {
remove(viewTag);
return;
}
runningAnimationIndicesMap_[viewTag].clear();
std::vector<size_t> updatedIndices;
for (const auto &[index, _] : newAnimations) {
updatedIndices.push_back(index);
}
for (const auto &[index, _] : settingsUpdates) {
updatedIndices.push_back(index);
}
updateAnimationSettings(animationsVector, settingsUpdates, timestamp);
for (size_t i = 0; i < animationsVector.size(); ++i) {
scheduleOrActivateAnimation(i, animationsVector[i], timestamp);
}
auto animationToIndexMap = buildAnimationToIndexMap(animationsVector);
registry_.erase(viewTag);
registry_.emplace(
viewTag,
RegistryEntry{
std::move(animationsVector), std::move(animationToIndexMap)});Basically, I just removed the const modifier from the animationsVector and moved the registry updates after updateAnimationSettings and scheduleOrActivateAnimation function calls, which are passed the animationsVector. I also added std::move to the animationToIndexMap which is now created before the animationsVector is moved to the RegistryEntry.
Let me know what do you think about this approach.
packages/react-native-reanimated/Common/cpp/reanimated/CSS/utils/DelayedItemsManager.cpp
Outdated
Show resolved
Hide resolved
…comply with the .h
2 participants
Summary
I uncommented
- performance-*clang-tidy check glob. This check encourages good performance practices. After that I solved CSA errors generated by the code.